Tidy MathListIndex by charlesroddie · Pull Request #159 · verybadcat/CSharpMath

charlesroddie · 2020-08-11T09:33:16Z

Currently MathListIndex is supposed to have a SubIndex if an only if SubIndexType <> MathListSubIndexType.None. (Can you confirm that @Happypig375 ?)

This PR brings this constraint into the type structure, removing MathListSubIndexType.None and having:

public (MathListSubIndexType SubIndexType, MathListIndex SubIndex)? SubIndexInfo;

CSharpMath.Editor/MathKeyboard.cs

Happypig375 · 2020-08-11T13:36:40Z

Yes.

charlesroddie · 2020-08-12T08:28:40Z

Getting errors I'm having trouble diagnosing.

Inputs: 1, power, 2

Debug code gives:

InsertAndAdvance index before: [0, Superscript:0]
InsertAndAdvance index before: [0]
InsertAtAtomIndexAndAdvance atom: Number 2
InsertAtAtomIndexAndAdvance advance before: [0]
InsertAtAtomIndexAndAdvance advance after: [1]
InsertAndAdvance index at end: [1]
InsertAndAdvance index at end: [0, Superscript:0] // should be [0, Superscript:1]

I have the feeling it's to do with mutability and the refs. I didn't consider the refs when editing.

static void InsertAtAtomIndexAndAdvance(this MathList self, int atomIndex, MathAtom atom, ref MathListIndex advance

Independently of getting this PR working, it's good to remove the refs and make MathList immutable. This would involve more recursive code. But it could be good to get this PR working before tackling that I think.

Once this PR goes in and the immutable change we will have a clean base to work on #117 .

Any thoughts @Happypig375 ?

Happypig375 · 2020-08-12T17:49:03Z

.editorconfig

 # https://docs.microsoft.com/en-us/visualstudio/ide/editorconfig-code-style-settings-reference#c-formatting-settings
 csharp_new_line_before_catch = false
-csharp_new_line_before_else = false
+#csharp_new_line_before_else = false


Happypig375 · 2020-08-12T17:49:12Z

.editorconfig

 csharp_new_line_before_members_in_object_initializers = false
 csharp_new_line_before_members_in_anonymous_types = false
-csharp_new_line_before_open_brace = none
+#csharp_new_line_before_open_brace = none


Happypig375 · 2020-08-12T17:52:25Z

CSharpMath.Editor/Extensions/MathList.cs

-        case MathListSubIndexType.Superscript:
-          self.Atoms[index.AtomIndex].Superscript.InsertAndAdvance(ref index.SubIndex, atom, advanceType);
+        case (MathListSubIndexType.Superscript, MathListIndex subIndex):
+          self.Atoms[index.AtomIndex].Superscript.InsertAndAdvance(ref subIndex, atom, advanceType);


The ref now mutates the local instead of the index. Same below.

Happypig375 · 2020-08-12T17:54:57Z

CSharpMath.Editor/SubIndexTypeMismatchException.cs

-      Array.IndexOf(typeof(MathListSubIndexType).GetEnumValues(), index.SubIndexType) == -1
-      ? $"{index.SubIndexType} is an invalid subindex type."
-      : $"{index.SubIndexType} not found at index {index.AtomIndex}.") { }
+      Array.IndexOf(typeof(MathListSubIndexType).GetEnumValues(), index.SubIndexInfo!.Value.SubIndexType) == -1


Exception will appear when SubIndexInfo is null

Happypig375 · 2020-08-12T17:55:14Z

NuGet.Config

+  <packageSources>
+    <add key="nuget.org" value="https://api.nuget.org/v3/index.json" protocolVersion="3" />
+  </packageSources>
+</configuration>


Happypig375 · 2020-08-12T17:56:14Z

CSharpMath.Editor/MathListIndex.cs

+
+    /// <summary>Factory function to create a `MathListIndex` with no subindexes.</summary>
+    /// <param name="index">The index of the atom that the `MathListIndex` points at.</param>
+    public static MathListIndex Level0Index(int index) => new MathListIndex(index, null);


Can be made into a default parameter of constructor

Happypig375 · 2020-08-12T17:58:15Z

Independently of getting this PR working, it's good to remove the refs

I don't think removing refs can be independent of this PR. Currently InsertAtAtomIndexAndAdvance advances the advance parameter and now that behaviour is gone, hence test failures.

charlesroddie · 2020-08-15T19:45:36Z

CSharpMath.Editor/Extensions/MathList.cs

+          return self.InsertAtAtomIndexAndAdvance(atomIndex, atom, advanceType);
+        case (MathListSubIndexType type, MathListIndex subIndex):
+          switch (type) {
+            case MathListSubIndexType.BetweenBaseAndScripts: // TODO: finish this section


I don't really understand this case. The docs on BetweenBaseAndScripts has The position in the subindex is an index into the nucleus, must be 1 and I don't understand that exactly. Could you help with this part @Happypig375 ?

The AtomIndex of the SubIndex must be 1 for SubIndexType BetweenBaseAndScripts.

I got that far...

In 12^3, is there a MathListIndex of 2?

Yes. The right boundary of the whole MathList. (for insertions)

What is the MathListIndex of 2 in 12^3?

Yes. The right boundary of the whole MathList. (for insertions)

Cursor position MathListIndex

‸12^3 0

1‸2^3 1

12‸^3 1, BetweenBaseAndScripts:1

12^‸3 1, Superscript:0

12^3‸ 1, Superscript:1 (to the right of 3) OR 2 (to the right of the entire MathList)

Happypig375 · 2026-02-17T20:30:45Z

@charlesroddie Review this updated version

Copilot

Pull request overview

This PR refactors the editor indexing model by removing MathListSubIndexType.None and representing “no subindex” structurally via a nullable (SubIndexType, SubIndex) tuple on MathListIndex. The change propagates through insertion/removal logic, hit-testing (IndexForPoint), caret movement, and related tests/public API.

Changes:

Replace “None vs non-None + nullable SubIndex” with MathListIndex(..., SubIndexInfo = null|(...)) and make MathListIndex a record.
Update editor operations (InsertAndAdvance, RemoveAt, cursor movement) and display hit-testing APIs to use the new nullable-subindex representation.
Update tests/namespaces and add IsExternalInit to support init on netstandard2.0.

Reviewed changes

Copilot reviewed 29 out of 29 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
CSharpMath/PublicAPI.Unshipped.txt	Records public surface changes for `MathListIndex`, hit-testing nullability, and enum/member changes.
CSharpMath/Editor/SubIndexTypeMismatchException.cs	Simplifies exception ctor to `(target, atomIndex)` to match refactored indexing.
CSharpMath/Editor/MathListRange.cs	Converts to `record struct` with `Start/Length` and updates subrange computation for new subindex model.
CSharpMath/Editor/MathListIndex.cs	Core refactor: remove `None`, add `SubIndexInfo`, convert to record, update navigation helpers.
CSharpMath/Editor/MathKeyboard.cs	Adapts caret movement and insertion/deletion flows to new `MathListIndex` APIs.
CSharpMath/Editor/Extensions/TextLineDisplay.cs	Updates point/index conversion and highlighting checks for new subindex model.
CSharpMath/Editor/Extensions/RadicalDisplay.cs	Makes `IndexForPoint` nullable and uses `Wrap` to build subindexes.
CSharpMath/Editor/Extensions/MathList.cs	Refactors insertion/removal helpers to return updated `MathListIndex` and use `SubIndexInfo`.
CSharpMath/Editor/Extensions/ListDisplay.cs	Updates hit-testing, recursion, and highlighting logic for new index representation.
CSharpMath/Editor/Extensions/LargeOpLimitsDisplay.cs	Makes `IndexForPoint` nullable and updates subindex wrapping logic.
CSharpMath/Editor/Extensions/InnerDisplay.cs	Makes `IndexForPoint` nullable and updates wrapping/validation.
CSharpMath/Editor/Extensions/IGlyphDisplay.cs	Updates “no subindex allowed” checks and index creation style.
CSharpMath/Editor/Extensions/FractionDisplay.cs	Makes `IndexForPoint` nullable and updates wrapping/validation.
CSharpMath/CSharpMath.csproj	Adds `IsExternalInit` for `init` support on `netstandard2.0`.
CSharpMath/Atom/MathList.cs	Minor XML-doc fix (`<see langword="null"/>`).
CSharpMath.Rendering.Tests/TestRendering.cs	Updates test helper namespace reference.
CSharpMath.Core.Tests/Editor/PointForIndexTests.cs	Moves tests to `EditorTests` namespace and updates using directives.
CSharpMath.Core.Tests/Editor/KeyPressTests.cs	Moves tests to `EditorTests` namespace.
CSharpMath.Core.Tests/Editor/IndexForPointTests.cs	Moves tests to `EditorTests` and updates index construction to `Wrap`/new model.
CSharpMath.Core.Tests/Editor/CaretTests.cs	Moves tests to `EditorTests` and updates index construction.
CSharpMath.Core.Tests/Display/TypesetterTests.cs	Moves tests to `DisplayTests` and updates parser test reference.
CSharpMath.Core.Tests/Display/MockTests.cs	Moves tests to `DisplayTests` namespace.
CSharpMath.Core.Tests/Display/FontChangingTests.cs	Moves tests to `DisplayTests` namespace.
CSharpMath.Core.Tests/Display/ApproximateAssertions.cs	Moves assertions to `DisplayTests` namespace.
CSharpMath.Core.Tests/Atom/MathListTest.cs	Moves tests to `AtomTests` namespace.
CSharpMath.Core.Tests/Atom/MathAtomTest.cs	Moves tests to `AtomTests` namespace.
CSharpMath.Core.Tests/Atom/LaTeXSettingsTests.cs	Moves tests to `AtomTests` namespace.
CSharpMath.Core.Tests/Atom/LaTeXParserTest.cs	Moves tests to `AtomTests` namespace.
CSharpMath.Core.Tests/Atom/DictionaryTests.cs	Moves tests to `AtomTests` namespace.

Comments suppressed due to low confidence (1)

CSharpMath/Editor/MathListIndex.cs:19

MathListSubIndexType is a public enum and removing None without explicitly preserving the existing numeric values shifts every underlying value down by 1. That’s a breaking change for any consumers persisting/serializing these values; consider assigning explicit values (leaving 0 unused) to keep the previous numbering stable.

  public enum MathListSubIndexType : byte {
    ///<summary>The position in the subindex is an index into the nucleus, must be 1</summary>
    BetweenBaseAndScripts,
    ///<summary>The subindex indexes into the superscript</summary>
    Superscript,
    ///<summary>The subindex indexes into the subscript</summary>
    Subscript,
    ///<summary>The subindex indexes into the numerator (only valid for fractions)</summary>
    Numerator,
    ///<summary>The subindex indexes into the denominator (only valid for fractions)</summary>
    Denominator,
    ///<summary>The subindex indexes into the radicand (only valid for radicals)</summary>
    Radicand,
    ///<summary>The subindex indexes into the degree (only valid for radicals)</summary>
    Degree,
    ///<summary>The subindex indexes into the inner list (only valid for inners)</summary>
    Inner

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

CSharpMath/Editor/MathListIndex.cs

CSharpMath/Editor/Extensions/MathList.cs

CSharpMath/Editor/Extensions/TextLineDisplay.cs

Copilot

Pull request overview

Copilot reviewed 29 out of 29 changed files in this pull request and generated 4 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

CSharpMath/Editor/MathListRange.cs

CSharpMath/Editor/MathListIndex.cs

CSharpMath/Editor/Extensions/MathList.cs

CSharpMath/Editor/Extensions/TextLineDisplay.cs

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 29 out of 29 changed files in this pull request and generated no new comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

charlesroddie changed the title ~~Tidy MathListIndex~~ [WIP] Tidy MathListIndex Aug 11, 2020

charlesroddie commented Aug 11, 2020

View reviewed changes

CSharpMath.Editor/MathKeyboard.cs Outdated Show resolved Hide resolved

Happypig375 reviewed Aug 12, 2020

View reviewed changes

charlesroddie commented Aug 15, 2020

View reviewed changes

Happypig375 added the Type/Housekeeping This includes internal only changes. label Oct 27, 2020

Happypig375 added this to the 1.0.0 Update Uno for .NET 10 milestone Feb 13, 2026

WIP rebased onto latest master

b35873a

Happypig375 force-pushed the TidyMathListIndex branch from 3936e72 to b35873a Compare February 16, 2026 21:13

Finalize

4f1af4a

github-actions bot added the Type/Enhancement label Feb 17, 2026

Happypig375 removed the Type/Housekeeping This includes internal only changes. label Feb 17, 2026

Happypig375 marked this pull request as ready for review February 17, 2026 20:28

Happypig375 changed the title ~~[WIP] Tidy MathListIndex~~ Tidy MathListIndex Feb 17, 2026

Happypig375 requested review from Copilot February 17, 2026 20:31

Copilot started reviewing on behalf of Happypig375 February 17, 2026 20:31 View session

Copilot AI reviewed Feb 17, 2026

View reviewed changes

CSharpMath/Editor/MathListRange.cs Outdated Show resolved Hide resolved

CSharpMath/Editor/MathListIndex.cs Show resolved Hide resolved

CSharpMath/Editor/Extensions/MathList.cs Outdated Show resolved Hide resolved

CSharpMath/Editor/Extensions/TextLineDisplay.cs Outdated Show resolved Hide resolved

Happypig375 and others added 4 commits February 18, 2026 05:01

Format again

e3acfa5

Update CSharpMath/Editor/MathListIndex.cs

1e0fd67

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Apply suggestions from code review

9d9a739

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Fix wrong condition

8e34856

Happypig375 requested a review from Copilot February 17, 2026 21:12

Copilot started reviewing on behalf of Happypig375 February 17, 2026 21:13 View session

Copilot AI reviewed Feb 17, 2026

View reviewed changes

Cursor position	MathListIndex
‸12^3	0
1‸2^3	1
12‸^3	1, BetweenBaseAndScripts:1
12^‸3	1, Superscript:0
12^3‸	1, Superscript:1 (to the right of 3) OR 2 (to the right of the entire MathList)

Conversation

charlesroddie commented Aug 11, 2020

Uh oh!

Uh oh!

Happypig375 commented Aug 11, 2020

Uh oh!

charlesroddie commented Aug 12, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Happypig375 commented Aug 12, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Happypig375 Aug 16, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Happypig375 Aug 16, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

charlesroddie Aug 16, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Happypig375 commented Feb 17, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Comments

Happypig375 Aug 16, 2020 •

edited

Loading

Happypig375 Aug 16, 2020 •

edited

Loading

charlesroddie Aug 16, 2020 •

edited

Loading